Repository navigation
fix(redshift): terminate running queries when SQLMesh is interrupted - #6144
Draft
gandeevanraghuramandd wants to merge 4 commits into
Draft
gandeevanraghuramandd wants to merge 4 commits into
gandeevanraghuramandd wants to merge 4 commits into
Conversation
On Ctrl-C, SnapshotEvaluator.concurrent_context() recycles the adapter before the context closes it. ThreadLocalConnectionPool.close_all() clears every thread's attributes, so close() no longer found the backend PIDs of queries still running on worker threads and left them running on Redshift. RedshiftEngineAdapter now terminates running queries in both recycle() and close(), before the pool is closed. If the calling thread was interrupted mid-query or mid-transaction, its connection is closed and the sessions are terminated from a new one. Also removes the SIGTERM-to-SIGINT mapping in the CLI, which is deferred to a follow-up so this change is scoped to Ctrl-C.
When the thread-local connection pools close another thread's connection while it's in the middle of a transaction (e.g. when Ctrl-C recycles the pool while workers are still running), they used to forget the transaction. The worker's next statement then ran on a new connection in a new transaction and could be committed without the statements that were rolled back with the closed connection. The transaction is now marked as broken instead. The worker's next statement fails without opening a new connection, its rollback doesn't touch the database, and committing it fails. Rollback also ends the transaction now even if it fails.
When SQLMesh is interrupted (e.g. by Ctrl-C), worker threads aren't waited for. A worker that hadn't started evaluating or creating a snapshot yet would carry on after teardown on a new connection and run the snapshot's statements anyway. concurrent_context() now sets an event when it's interrupted, and workers check it before a snapshot's non-transactional pre-statements and before its main transaction. Work after the main transaction commits isn't stopped, so the interval of committed data is still recorded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On Ctrl-C during a concurrent run, SQLMesh exits without stopping the queries its worker threads started. Redshift keeps running them after the client disconnects, holding their locks until they finish. The workers also keep going after teardown and can commit part of a model on a new connection.
This PR:
pg_terminate_backend) inrecycle()andclose()Test Plan
Added unit tests for each change. Not yet tested against a real Redshift cluster.
Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCOMade with Cursor